Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

22 new data source security account #114

Merged
merged 15 commits into from
Jan 24, 2024

Conversation

carchi8py
Copy link
Contributor

Adds #22

@carchi8py carchi8py linked an issue Dec 12, 2023 that may be closed by this pull request
"github.com/netapp/terraform-provider-netapp-ontap/internal/utils"
)

// TODO:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines can be removed.

MarkdownDescription: "SecurityAccount id",
Computed: true,
},
"owner": schema.SingleNestedAttribute{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'owner' should be required in one data source case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NO.... YES.....

So an account can be SVM specific or cluster specific.

Let me try some test out, my worry here is cluster name might not be known by the user.

"github.com/netapp/terraform-provider-netapp-ontap/internal/utils"
)

// TODO:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these lines can be removed


Required:

- `name` (String) SecurityAccount owner name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user only provides "name", then it will be multiple records.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there a way around that.

Copy link
Contributor

@chuyich chuyich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question on the single data source: the user needs to provide name and owner info to get the single record. So owner probably needs to be required.

Copy link
Contributor

@chuyich chuyich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also missing the CHANGELOG update

@carchi8py
Copy link
Contributor Author

One question on the single data source: the user needs to provide name and owner info to get the single record. So owner probably needs to be required.

Since cluster user can't give an owner info we can't have it required.

#export TF_ACC_NETAPP_USER="admin"
#export TF_ACC_NETAPP_PASS="<password>"
#export TF_ACC_NETAPP_LICENSE="<licensekey>"
export TF_ACC_NETAPP_HOST="10.193.180.108"
Copy link
Contributor

@chuyich chuyich Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok that we hard-coded these values?

@chuyich
Copy link
Contributor

chuyich commented Jan 24, 2024

Please resolve the conflict, then it should be good to go.

@carchi8py carchi8py merged commit 24937ec into integration/main Jan 24, 2024
7 checks passed
@carchi8py carchi8py deleted the 22-new-data-source-security_account branch January 24, 2024 22:33
acch pushed a commit to acch/terraform-provider-netapp-ontap that referenced this pull request Jan 22, 2025
…_account

22 new data source security account
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Data Source]: security_account
2 participants